feat: Add player age eligibility validation for leagues#38
feat: Add player age eligibility validation for leagues#38
Conversation
Add functions to validate if a player is eligible to participate in a league by comparing their dateOfBirth against the league's minAge and maxAge restrictions: - calculateAge: Utility to compute age from date of birth - checkPlayerEligibility: Returns validation result with errors - validatePlayerEligibility: Throws error if ineligible - isPlayerEligible: Returns boolean for eligibility Includes comprehensive tests for all edge cases including boundary conditions, missing data, and invalid formats.
|
⏳ I'm reviewing this pull request for security vulnerabilities and code quality issues. I'll provide an update when I'm done |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (2)
📝 WalkthroughWalkthroughThis PR adds four new age-based validation utilities to Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant ValidateFunc as validatePlayerEligibility()
participant CheckFunc as checkPlayerEligibility()
participant CalcFunc as calculateAge()
Client->>ValidateFunc: (dateOfBirth, minAge, maxAge, referenceDate)
ValidateFunc->>CheckFunc: (dateOfBirth, minAge, maxAge, referenceDate)
CheckFunc->>CalcFunc: (dateOfBirth, referenceDate)
CalcFunc-->>CheckFunc: age (number | null)
alt Valid input & within constraints
CheckFunc-->>ValidateFunc: ValidationCheckResult (valid)
ValidateFunc-->>Client: returns void (success)
else Invalid input or out-of-bounds
CheckFunc-->>ValidateFunc: ValidationCheckResult (errors)
ValidateFunc->>Client: throws Error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
✅ I finished the code review, and didn't find any security or code quality issues. |
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
test/validation.test.ts (1)
967-1006: Good coverage, but missing timezone edge case tests.The test suite covers the main scenarios well. However, it doesn't expose the timezone bug in
calculateAgebecause all tests use explicit date strings forreferenceDate(which are parsed as UTC, matching the UTC parsing ofdateOfBirth).Consider adding a test that uses the current time in a specific timezone to ensure the fix handles timezone correctly:
it('should handle timezone correctly when using current time', () => { // Test that would expose the timezone bug if not using UTC methods const birthDate = '2000-01-01'; const age = calculateAge(birthDate, new Date(2024, 0, 1, 12, 0, 0)); // January 1, 2024 noon local time expect(age).toBe(24); });This test would fail with the current implementation in timezones west of UTC.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
docs/assets/navigation.js(1 hunks)docs/assets/search.js(1 hunks)docs/functions/calculateAge.html(1 hunks)docs/functions/checkPlayerEligibility.html(1 hunks)docs/functions/isPlayerEligible.html(1 hunks)docs/functions/validatePlayerEligibility.html(1 hunks)src/validation.ts(1 hunks)test/validation.test.ts(2 hunks)
🔇 Additional comments (12)
docs/assets/navigation.js (1)
1-1: LGTM - Generated documentation artifact.This is a generated navigation data payload that reflects the new API surface. No action required.
docs/assets/search.js (1)
1-1: LGTM - Generated documentation artifact.This is a generated search index that reflects the new API surface. No action required.
docs/functions/calculateAge.html (1)
1-5: LGTM - Generated documentation page.The documentation accurately describes the
calculateAgefunction's parameters and return type. No action required.docs/functions/checkPlayerEligibility.html (1)
1-7: LGTM - Generated documentation page.The documentation accurately describes the
checkPlayerEligibilityfunction's parameters and return type. No action required.docs/functions/validatePlayerEligibility.html (1)
1-7: LGTM - Generated documentation page.The documentation accurately describes the
validatePlayerEligibilityfunction's throwing behavior. No action required.docs/functions/isPlayerEligible.html (1)
1-7: LGTM - Generated documentation page.The documentation accurately describes the
isPlayerEligiblefunction's boolean return value. No action required.src/validation.ts (3)
523-560: LGTM - Solid validation logic.The age eligibility validation correctly handles all edge cases:
- Returns no errors when no age restrictions are set
- Validates that dateOfBirth is provided when restrictions exist
- Delegates to
calculateAgefor date parsing and age computation- Checks both minimum and maximum age constraints with clear error messages
The use of
!= nullproperly catches both null and undefined values.Note: This function depends on
calculateAge, which has a timezone issue flagged separately.
570-578: LGTM - Clean throwing wrapper.The function follows the established validation pattern in the codebase by delegating to
checkPlayerEligibilityand throwing viaresult.validate().
588-596: LGTM - Useful boolean convenience method.The function provides a clean boolean API by delegating to
checkPlayerEligibilityand converting the result to true/false based on error presence.test/validation.test.ts (3)
1008-1138: LGTM - Comprehensive test coverage.The test suite for
checkPlayerEligibilityis excellent, covering:
- No restrictions scenario
- Missing/invalid date of birth handling
- Minimum and maximum age boundaries
- Combined age range validation
- Edge cases (exact age requirements, zero minimum age)
The tests verify both the absence of errors in valid cases and the presence of specific error messages in invalid cases.
1140-1165: LGTM - Adequate coverage for throwing wrapper.The tests verify the key throwing behaviors:
- No exception for eligible players
- Correct exception messages for ineligible players (too young, too old, missing DOB)
- No exception when no restrictions exist
1167-1197: LGTM - Complete boolean wrapper coverage.The tests verify the boolean return values across all scenarios:
- True for eligible players
- False for all ineligibility reasons
- Partial restriction handling (minAge-only, maxAge-only)
Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
3 participants
Add functions to validate if a player is eligible to participate in a league by comparing their dateOfBirth against the league's minAge and maxAge restrictions:
Includes comprehensive tests for all edge cases including boundary conditions, missing data, and invalid formats.
Summary by CodeRabbit
New Features
Documentation
Content Updates